Skip to content

feat(auth): add email verification and resend verification flow#40

Merged
aniebietafia merged 4 commits intomainfrom
feat/email-verification
Mar 19, 2026
Merged

feat(auth): add email verification and resend verification flow#40
aniebietafia merged 4 commits intomainfrom
feat/email-verification

Conversation

@aniebietafia
Copy link
Contributor

@aniebietafia aniebietafia commented Mar 17, 2026

Summary by CodeRabbit

  • New Features

    • Email verification flow: signup issues persistent verification tokens, verify-email endpoint, and resend-verification with rate limits; DB migration adds token storage.
  • Documentation

    • API docs for verification endpoints and new "Logging Safety" guidance.
  • Tests

    • Comprehensive unit and integration tests covering verification, token CRUD, and sanitization.
  • Chores

    • New config for token expiry window and widespread logging-sanitization plus global rate-limiter wiring.

add VerificationToken model and migration for persistent token storage

implement class-based verification logic via VerificationTokenRepository and AuthVerificationService

add GET /api/v1/auth/verify-email with token validation, expiry handling, and idempotent verified-user behavior

add POST /api/v1/auth/resend-verification with enumeration-safe response and 3/minute rate limit

integrate signup to generate verification tokens and enqueue email dispatch via Kafka email producer

add standardized 429 error handling through custom rate-limit exception handler

document endpoints in docs/auth_verification_api.md

add tests for verification CRUD and API flows (success, missing token, invalid token, expired token, resend cases)

keep code quality gates compliant (ruff, isort, mypy) and auth tests green

Signed-off-by: aniebietafia <[email protected]>
add VerificationToken model and migration for persistent token storage

implement class-based verification logic via VerificationTokenRepository and AuthVerificationService

add GET /api/v1/auth/verify-email with token validation, expiry handling, and idempotent verified-user behavior

add POST /api/v1/auth/resend-verification with enumeration-safe response and 3/minute rate limit

integrate signup to generate verification tokens and enqueue email dispatch via Kafka email producer

add standardized 429 error handling through custom rate-limit exception handler

document endpoints in docs/auth_verification_api.md

add tests for verification CRUD and API flows (success, missing token, invalid token, expired token, resend cases)

keep code quality gates compliant (ruff, isort, mypy) and auth tests green

Signed-off-by: aniebietafia <[email protected]>
@coderabbitai
Copy link

coderabbitai bot commented Mar 17, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 052c34c4-aa14-4b2d-bfda-5f1e2e468820

📥 Commits

Reviewing files that changed from the base of the PR and between cc2bef8 and 1c0b9bf.

📒 Files selected for processing (1)
  • app/api/v1/endpoints/auth.py

📝 Walkthrough

Walkthrough

Adds an email verification system: persistent verification tokens, verification/resend endpoints (rate-limited), token CRUD, service layer, model + migration, logging sanitization, rate limiter wiring, and tests.

Changes

Cohort / File(s) Summary
Env & Config
\.env\.example, app/core/config.py
Adds VERIFICATION_TOKEN_EXPIRE_HOURS setting (default 24).
Models & Migration
app/models/verification_token.py, app/models/__init__.py, alembic/versions/19dc9714d9ea_add_email_verification_model.py
New VerificationToken model and Alembic migration creating verification_tokens table with indexes and FK to users.
CRUD / Repository
app/crud/verification_token.py, tests/test_auth/test_verification_token_crud.py
New VerificationTokenRepository with get/create/delete/prune methods and unit tests for lifecycle and expiry behavior.
Service Layer
app/services/auth_verification.py
New AuthVerificationService: create tokens, verify tokens (format/expiry), mark user verified, resend verification (async email enqueue), and DI factory/singleton.
API Endpoints & Docs
app/api/v1/endpoints/auth.py, app/crud/auth_verification_api.md
Signup now creates persistent verification tokens and enqueues email; adds GET /auth/verify-email and POST /auth/resend-verification (rate-limited) endpoints and API docs.
Rate Limiting
app/core/rate_limiter.py, app/main.py
Adds SlowAPI limiter instance, 429 handler, and wires limiter into FastAPI app state and exception handling.
Logging Sanitization
app/core/sanitize.py, app/core/exception_handlers.py, app/kafka/..., app/services/email_consumer.py, app/services/email_producer.py
Introduces LogSanitizer and applies sanitized logging across Kafka and email modules; exception handler logs sanitized errors.
Email Producer / Consumer
app/kafka/producer.py, app/kafka/consumer.py, app/kafka/manager.py, app/services/email_consumer.py, app/services/email_producer.py
Switches to parameterized logging and sanitizes dynamic log args; no behavior changes to Kafka/email flows.
Tests
tests/test_auth/test_email_verification.py, tests/test_core/test_sanitize.py
Adds integration tests for verification flows (signup, verify, resend, error cases) and unit tests for sanitization behavior.
Docs
README.md
Adds "Logging Safety" section describing sanitization APIs and behavior.

Sequence Diagram(s)

sequenceDiagram
    actor Client
    participant AuthAPI as Auth Endpoint
    participant AuthService as AuthVerification Service
    participant TokenRepo as Token Repository
    participant DB as Database
    participant EmailProducer as Email Producer
    participant Kafka as Kafka
    participant EmailConsumer as Email Consumer

    rect rgba(100,200,100,0.5)
    Client->>AuthAPI: POST /api/v1/auth/signup
    AuthAPI->>AuthService: create_verification_token(user_id)
    AuthService->>TokenRepo: create_token(user_id)
    TokenRepo->>DB: INSERT verification_token
    TokenRepo-->>AuthService: VerificationToken
    AuthService-->>AuthAPI: token
    AuthAPI->>EmailProducer: send_email(template=verify,to=email)
    EmailProducer->>Kafka: enqueue(topic=notifications.email)
    AuthAPI-->>Client: 200 SignupResponse
    end

    rect rgba(100,150,200,0.5)
    Client->>AuthAPI: GET /api/v1/auth/verify-email?token=...
    AuthAPI->>AuthService: verify_email(token)
    AuthService->>TokenRepo: get_token(token)
    TokenRepo->>DB: SELECT verification_token
    TokenRepo-->>AuthService: VerificationToken
    AuthService->>DB: UPDATE users SET is_verified=true
    AuthService->>TokenRepo: delete_token(token_id)
    TokenRepo->>DB: DELETE verification_token
    AuthService-->>AuthAPI: success
    AuthAPI-->>Client: 200 VerifyEmailResponse
    end

    rect rgba(200,150,100,0.5)
    Client->>AuthAPI: POST /api/v1/auth/resend-verification
    AuthAPI->>AuthService: resend_verification_email(email)
    AuthService->>DB: SELECT user WHERE email
    alt user exists && not verified
      AuthService->>TokenRepo: delete_unexpired_tokens_for_user(user_id)
      AuthService->>TokenRepo: create_token(user_id)
      AuthService->>EmailProducer: send_email(template=verify,to=email)
      EmailProducer->>Kafka: enqueue(topic=notifications.email)
    end
    AuthService-->>AuthAPI: ok
    AuthAPI-->>Client: 200 ActionAcknowledgement
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related issues

  • Issue 27: Implements the verify-email and resend-verification endpoints, token model, repository, and service described in this PR.

Possibly related PRs

  • PR 40: Implements the same email verification feature (model, CRUD, service, endpoints, env var, migration, rate limiting, tests).
  • PR 38: Modifies signup/forgot-password flows and integrates EmailProducerService/Kafka email queuing relevant to these changes.
  • PR 37: Alters auth signup wiring and verification-token logic overlapping with this PR's endpoint/signup changes.

Poem

🐰 A token hops from code to mail,

Safe links stitched in a careful trail,
Rate limits guard the hopping race,
Logs stay tidy, trimmed with grace,
Click once — the inbox finds its place.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 19.44% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title 'feat(auth): add email verification and resend verification flow' accurately and comprehensively describes the main changes: introducing email verification endpoints, token management, and a resend mechanism across the auth service.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/email-verification
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@aniebietafia aniebietafia linked an issue Mar 17, 2026 that may be closed by this pull request
9 tasks
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 5

🧹 Nitpick comments (3)
app/models/verification_token.py (1)

10-11: Avoid a second source of truth for token expiry.

app/crud/verification_token.py already computes expires_at from settings.VERIFICATION_TOKEN_EXPIRE_HOURS, so this hard-coded 24-hour fallback is dead today. If any future insert path relies on the model default, it will silently ignore the configured TTL.

Also applies to: 26-28

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/models/verification_token.py` around lines 10 - 11, The model-level
default_expiry() currently hardcodes a 24-hour TTL which duplicates and can
contradict the TTL computed in app/crud/verification_token.py; update
default_expiry() to derive its timedelta from
settings.VERIFICATION_TOKEN_EXPIRE_HOURS (import settings) so the model default
and CRUD insertion use the same single source of truth for expires_at, or remove
the model default entirely and rely solely on the CRUD layer—adjust the
expires_at field and any references to default_expiry accordingly (see
default_expiry, expires_at and app/crud/verification_token.py).
app/crud/auth_verification_api.md (1)

82-83: Clarify the idempotency note.

Successful verification deletes the token, so the same link is not 200-safe after first use; it falls through to INVALID_TOKEN. Please narrow this note to the case where the user is already verified and the token row still exists.

📝 Suggested wording
-- Already verified users are handled idempotently by `GET /verify-email` and receive `200`.
+- If the user is already verified and the token row still exists, `GET /verify-email` returns `200`.
+- After a successful verification consumes the token, the same link returns `INVALID_TOKEN`.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/crud/auth_verification_api.md` around lines 82 - 83, Update the
idempotency note for GET /verify-email to clarify that tokens are single-use and
are deleted upon successful verification so reusing the same link will typically
yield INVALID_TOKEN; only treat the request as idempotent (returning 200) in the
specific case where the user is already verified and the token row still exists.
Replace the existing second bullet with wording that explicitly states: "Already
verified users are handled idempotently by GET /verify-email only when the
verification token row still exists; otherwise a reused link will result in
INVALID_TOKEN." Ensure references to "successful verification deletes the
token", "GET /verify-email", and "INVALID_TOKEN" remain in the doc so the
behavior is clear.
tests/test_auth/test_email_verification.py (1)

167-193: Add the resend failure-path regression test.

This suite covers the happy path, but not the case where send_email() raises after a resend. That is the path currently able to invalidate the old token while the API still returns 200, so it is worth pinning once the service logic is adjusted.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_auth/test_email_verification.py` around lines 167 - 193, Add a
regression test that simulates send_email raising and asserts the old
verification token is NOT invalidated: create a test (e.g.,
test_resend_verification_failure_does_not_invalidate_token) that uses the same
helpers (_create_unverified_user and _get_verification_token), capture
old_value, set email_producer_mock.send_email.side_effect = Exception("send
failure"), call POST /api/v1/auth/resend-verification, assert
email_producer_mock.send_email was awaited once, and then fetch the token via
_get_verification_token and assert its token equals the previously captured
old_value to ensure a failed send does not rotate the token.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@alembic/versions/4b4b6b5d1c2a_add_verification_tokens_table.py`:
- Around line 32-43: Replace the redundant single-column index
ix_verification_tokens_id with a composite index on (user_id, expires_at) so
queries like delete_unexpired_tokens_for_user (in
app/crud/verification_token.py) can use an index; specifically, remove the
op.create_index call that creates ix_verification_tokens_id and instead add
op.create_index(op.f("ix_verification_tokens_user_id_expires_at"),
"verification_tokens", ["user_id", "expires_at"], unique=False). Make the same
replacement for the corresponding index in the later block (the second
occurrence around lines 47-50) so both upgrade and downgrade/other index
definitions reflect the composite index. Ensure ix_verification_tokens_token
(unique on token) remains unchanged.

In `@app/api/v1/endpoints/auth.py`:
- Around line 193-202: The current broad except around
auth_verification_service.resend_verification_email masks DB/token failures;
narrow the error handling so persistence errors propagate and only
email-producer failures are caught. Move the try/except so it surrounds just the
call to the email_producer (or have resend_verification_email raise a specific
EmailProducerError), catch that specific exception (e.g., EmailProducerError or
the producer's raised exception) and log via logger.warning("Failed to enqueue
verification resend for %s: %s", payload.email, exc); do not catch Exception
from within the overall resend_verification_email call so lookup/token mutation
errors surface as failures.

In `@app/crud/verification_token.py`:
- Around line 15-23: The create_token method (and the similar
revoke/current-token helper at lines 32-42) commits independently which can
cause a gap when called back-to-back from app/services/auth_verification.py;
change these helpers to support a no-commit mode (e.g. add an optional commit:
bool = True parameter or provide internal variants like _create_token_no_commit
and _revoke_current_tokens_no_commit) so they perform db.add/db.flush/db.refresh
without db.commit when commit is False, and then update the service layer to
call both helpers and perform a single db.commit() after both have succeeded;
reference the create_token function and the revoke_current_tokens helper when
implementing this change.

In `@app/services/auth_verification.py`:
- Around line 87-99: Don't delete unexpired tokens before emailing — instead
reuse an existing token if present or create a new one, then enqueue the email,
and only remove other tokens after send_email succeeds. Concretely: use the
repository method that returns existing tokens (e.g.,
get_unexpired_tokens_for_user) to pick/reuse a token; if none exists call
create_token to make one; build verification_link from that token; await
email_producer.send_email(...); only after send_email resolves successfully call
delete_unexpired_tokens_for_user (or a repository method that deletes other
tokens while preserving the just-sent token) to remove stale tokens.

In `@tests/test_auth/test_email_verification.py`:
- Around line 113-116: Replace the hard-coded UUID string in
test_verify_email_invalid_token_returns_custom_error with a programmatically
generated UUID (using uuid.uuid4()) so the request to
"/api/v1/auth/verify-email?token=..." still exercises the INVALID_TOKEN branch
without committing a literal that trips Gitleaks; update the test's client.get
call to interpolate uuid.uuid4().hex or str(uuid.uuid4()) for the token and
import uuid at the top of the test file if not already present.

---

Nitpick comments:
In `@app/crud/auth_verification_api.md`:
- Around line 82-83: Update the idempotency note for GET /verify-email to
clarify that tokens are single-use and are deleted upon successful verification
so reusing the same link will typically yield INVALID_TOKEN; only treat the
request as idempotent (returning 200) in the specific case where the user is
already verified and the token row still exists. Replace the existing second
bullet with wording that explicitly states: "Already verified users are handled
idempotently by GET /verify-email only when the verification token row still
exists; otherwise a reused link will result in INVALID_TOKEN." Ensure references
to "successful verification deletes the token", "GET /verify-email", and
"INVALID_TOKEN" remain in the doc so the behavior is clear.

In `@app/models/verification_token.py`:
- Around line 10-11: The model-level default_expiry() currently hardcodes a
24-hour TTL which duplicates and can contradict the TTL computed in
app/crud/verification_token.py; update default_expiry() to derive its timedelta
from settings.VERIFICATION_TOKEN_EXPIRE_HOURS (import settings) so the model
default and CRUD insertion use the same single source of truth for expires_at,
or remove the model default entirely and rely solely on the CRUD layer—adjust
the expires_at field and any references to default_expiry accordingly (see
default_expiry, expires_at and app/crud/verification_token.py).

In `@tests/test_auth/test_email_verification.py`:
- Around line 167-193: Add a regression test that simulates send_email raising
and asserts the old verification token is NOT invalidated: create a test (e.g.,
test_resend_verification_failure_does_not_invalidate_token) that uses the same
helpers (_create_unverified_user and _get_verification_token), capture
old_value, set email_producer_mock.send_email.side_effect = Exception("send
failure"), call POST /api/v1/auth/resend-verification, assert
email_producer_mock.send_email was awaited once, and then fetch the token via
_get_verification_token and assert its token equals the previously captured
old_value to ensure a failed send does not rotate the token.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: ca2c2dd1-6f7e-4303-a531-88580cbf3fed

📥 Commits

Reviewing files that changed from the base of the PR and between d582054 and 6428617.

📒 Files selected for processing (14)
  • .env.example
  • alembic/versions/4b4b6b5d1c2a_add_verification_tokens_table.py
  • app/api/v1/endpoints/auth.py
  • app/core/config.py
  • app/core/rate_limiter.py
  • app/crud/auth_verification_api.md
  • app/crud/verification_token.py
  • app/main.py
  • app/models/__init__.py
  • app/models/verification_token.py
  • app/schemas/auth.py
  • app/services/auth_verification.py
  • tests/test_auth/test_email_verification.py
  • tests/test_auth/test_verification_token_crud.py

Comment on lines +32 to +43
op.create_index(
op.f("ix_verification_tokens_id"),
"verification_tokens",
["id"],
unique=False,
)
op.create_index(
op.f("ix_verification_tokens_token"),
"verification_tokens",
["token"],
unique=True,
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
# Verify repository query patterns that filter by user_id/expires_at for verification tokens.
fd 'verification_token\.py$' app tests | xargs -r rg -n -C2 'user_id|expires_at|delete_unexpired_tokens_for_user|get_token\('

Repository: Brints/FluentMeet

Length of output: 3144


🏁 Script executed:

cat -n alembic/versions/4b4b6b5d1c2a_add_verification_tokens_table.py

Repository: Brints/FluentMeet

Length of output: 1916


Replace redundant id index with composite index supporting user-based token cleanup.

The index on id at line 32 is redundant—primary keys automatically have index support. More critically, the delete_unexpired_tokens_for_user() method in app/crud/verification_token.py:32-37 filters by both user_id and expires_at, but no index covers this pattern, causing full table scans as data grows.

Replace the id index with a composite index on (user_id, expires_at):

Suggested migration adjustment
 def upgrade() -> None:
     op.create_index(
-        op.f("ix_verification_tokens_id"),
+        op.f("ix_verification_tokens_user_id_expires_at"),
         "verification_tokens",
-        ["id"],
+        ["user_id", "expires_at"],
         unique=False,
     )

 def downgrade() -> None:
     op.drop_index(
         op.f("ix_verification_tokens_token"), table_name="verification_tokens"
     )
     op.drop_index(
-        op.f("ix_verification_tokens_id"), table_name="verification_tokens"
+        op.f("ix_verification_tokens_user_id_expires_at"), table_name="verification_tokens"
     )
     op.drop_table("verification_tokens")

Also applies to: 47-50

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@alembic/versions/4b4b6b5d1c2a_add_verification_tokens_table.py` around lines
32 - 43, Replace the redundant single-column index ix_verification_tokens_id
with a composite index on (user_id, expires_at) so queries like
delete_unexpired_tokens_for_user (in app/crud/verification_token.py) can use an
index; specifically, remove the op.create_index call that creates
ix_verification_tokens_id and instead add
op.create_index(op.f("ix_verification_tokens_user_id_expires_at"),
"verification_tokens", ["user_id", "expires_at"], unique=False). Make the same
replacement for the corresponding index in the later block (the second
occurrence around lines 47-50) so both upgrade and downgrade/other index
definitions reflect the composite index. Ensure ix_verification_tokens_token
(unique on token) remains unchanged.

Comment on lines +193 to +202
try:
await auth_verification_service.resend_verification_email(
db=db,
email=str(payload.email),
email_producer=email_producer,
)
except Exception as exc:
logger.warning(
"Failed to enqueue verification resend for %s: %s", payload.email, exc
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Don't mask token/database failures as a successful resend.

resend_verification_email() does user lookup and token mutation before it touches the email producer. This except Exception converts failures in those DB steps into a 200 "If an account..." response, which hides outages and can leave state partially changed. Catch only the producer failure path, or let the persistence errors propagate.

🧰 Tools
🪛 GitHub Check: CodeQL

[failure] 201-201: Log Injection
This log entry depends on a user-provided value.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/api/v1/endpoints/auth.py` around lines 193 - 202, The current broad
except around auth_verification_service.resend_verification_email masks DB/token
failures; narrow the error handling so persistence errors propagate and only
email-producer failures are caught. Move the try/except so it surrounds just the
call to the email_producer (or have resend_verification_email raise a specific
EmailProducerError), catch that specific exception (e.g., EmailProducerError or
the producer's raised exception) and log via logger.warning("Failed to enqueue
verification resend for %s: %s", payload.email, exc); do not catch Exception
from within the overall resend_verification_email call so lookup/token mutation
errors surface as failures.

Comment on lines +15 to +23
def create_token(self, db: Session, user_id: int) -> VerificationToken:
expires_at = datetime.now(UTC) + timedelta(
hours=settings.VERIFICATION_TOKEN_EXPIRE_HOURS
)
verification_token = VerificationToken(user_id=user_id, expires_at=expires_at)
db.add(verification_token)
db.commit()
db.refresh(verification_token)
return verification_token
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Let resend own the transaction boundary.

In app/services/auth_verification.py, Lines 87-88 call these helpers back-to-back during resend. Because both methods commit independently, a failure between them can revoke the user's current valid token before the replacement exists. Expose no-commit variants or move the single commit() up to the service layer.

Also applies to: 32-42

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/crud/verification_token.py` around lines 15 - 23, The create_token method
(and the similar revoke/current-token helper at lines 32-42) commits
independently which can cause a gap when called back-to-back from
app/services/auth_verification.py; change these helpers to support a no-commit
mode (e.g. add an optional commit: bool = True parameter or provide internal
variants like _create_token_no_commit and _revoke_current_tokens_no_commit) so
they perform db.add/db.flush/db.refresh without db.commit when commit is False,
and then update the service layer to call both helpers and perform a single
db.commit() after both have succeeded; reference the create_token function and
the revoke_current_tokens helper when implementing this change.

Comment on lines +87 to +99
self._token_repository.delete_unexpired_tokens_for_user(db=db, user_id=user.id)
token = self._token_repository.create_token(db=db, user_id=user.id)

verification_link = (
f"{settings.FRONTEND_BASE_URL}/verify-email?token={token.token}"
)
await email_producer.send_email(
to=user.email,
subject="Verify your FluentMeet account",
html_body=None,
template_data={"verification_link": verification_link},
template="verification",
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

Keep the current link valid until the replacement is queued.

Line 87 deletes the current unexpired token before Line 93 tries to enqueue the replacement email. If send_email() fails, the endpoint still returns 200 and the user loses the only verification link they already had. Reuse the existing token or delete the old one only after queueing succeeds.

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@app/services/auth_verification.py` around lines 87 - 99, Don't delete
unexpired tokens before emailing — instead reuse an existing token if present or
create a new one, then enqueue the email, and only remove other tokens after
send_email succeeds. Concretely: use the repository method that returns existing
tokens (e.g., get_unexpired_tokens_for_user) to pick/reuse a token; if none
exists call create_token to make one; build verification_link from that token;
await email_producer.send_email(...); only after send_email resolves
successfully call delete_unexpired_tokens_for_user (or a repository method that
deletes other tokens while preserving the just-sent token) to remove stale
tokens.

Comment on lines +113 to +116
def test_verify_email_invalid_token_returns_custom_error(client: TestClient) -> None:
response = client.get(
"/api/v1/auth/verify-email?token=8f14e45f-ceea-4f6a-9fef-3d4d3e0d1be1"
)
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor

Replace the UUID literal that trips Gitleaks.

The scanner is flagging this hard-coded value as a generic secret. uuid4() exercises the same INVALID_TOKEN branch without adding noisy security findings.

🧪 Suggested change
+from uuid import uuid4
 ...
 def test_verify_email_invalid_token_returns_custom_error(client: TestClient) -> None:
-    response = client.get(
-        "/api/v1/auth/verify-email?token=8f14e45f-ceea-4f6a-9fef-3d4d3e0d1be1"
-    )
+    response = client.get(f"/api/v1/auth/verify-email?token={uuid4()}")
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def test_verify_email_invalid_token_returns_custom_error(client: TestClient) -> None:
response = client.get(
"/api/v1/auth/verify-email?token=8f14e45f-ceea-4f6a-9fef-3d4d3e0d1be1"
)
from uuid import uuid4
def test_verify_email_invalid_token_returns_custom_error(client: TestClient) -> None:
response = client.get(f"/api/v1/auth/verify-email?token={uuid4()}")
🧰 Tools
🪛 Gitleaks (8.30.0)

[high] 115-115: Detected a Generic API Key, potentially exposing access to various services and sensitive operations.

(generic-api-key)

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@tests/test_auth/test_email_verification.py` around lines 113 - 116, Replace
the hard-coded UUID string in
test_verify_email_invalid_token_returns_custom_error with a programmatically
generated UUID (using uuid.uuid4()) so the request to
"/api/v1/auth/verify-email?token=..." still exercises the INVALID_TOKEN branch
without committing a literal that trips Gitleaks; update the test's client.get
call to interpolate uuid.uuid4().hex or str(uuid.uuid4()) for the token and
import uuid at the top of the test file if not already present.

add VerificationToken model and migration for persistent token storage

implement class-based verification logic via VerificationTokenRepository and AuthVerificationService

add GET /api/v1/auth/verify-email with token validation, expiry handling, and idempotent verified-user behavior

add POST /api/v1/auth/resend-verification with enumeration-safe response and 3/minute rate limit

integrate signup to generate verification tokens and enqueue email dispatch via Kafka email producer

add standardized 429 error handling through custom rate-limit exception handler

document endpoints in docs/auth_verification_api.md

add tests for verification CRUD and API flows (success, missing token, invalid token, expired token, resend cases)

keep code quality gates compliant (ruff, isort, mypy) and auth tests green

Signed-off-by: aniebietafia <[email protected]>
import sqlalchemy as sa

# revision identifiers, used by Alembic.
revision: str = "19dc9714d9ea"

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'revision' is not used.

Copilot Autofix

AI about 5 hours ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.


# revision identifiers, used by Alembic.
revision: str = "19dc9714d9ea"
down_revision: Union[str, Sequence[str], None] = "11781e907181"

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'down_revision' is not used.

Copilot Autofix

AI about 5 hours ago

Copilot could not generate an autofix suggestion

Copilot could not generate an autofix suggestion for this alert. Try pushing a new commit or if the problem persists contact support.

# revision identifiers, used by Alembic.
revision: str = "19dc9714d9ea"
down_revision: Union[str, Sequence[str], None] = "11781e907181"
branch_labels: Union[str, Sequence[str], None] = None

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'branch_labels' is not used.

Copilot Autofix

AI about 6 hours ago

To fix the reported issue without changing runtime behavior, we should indicate that branch_labels (and the other Alembic metadata symbols) are intentionally exported for external use. The simplest, non-invasive way is to define a module-level __all__ list that includes these names. Static analyzers then treat them as intentionally public, not unused.

Concretely, in alembic/versions/19dc9714d9ea_add_email_verification_model.py, right after the existing Alembic metadata assignments (revision, down_revision, branch_labels, depends_on), add a __all__ definition listing these four names. No imports are needed, and no existing lines must be altered or removed; we only insert new lines defining __all__. This preserves all current functionality while satisfying the static analysis rule.

Suggested changeset 1
alembic/versions/19dc9714d9ea_add_email_verification_model.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/alembic/versions/19dc9714d9ea_add_email_verification_model.py b/alembic/versions/19dc9714d9ea_add_email_verification_model.py
--- a/alembic/versions/19dc9714d9ea_add_email_verification_model.py
+++ b/alembic/versions/19dc9714d9ea_add_email_verification_model.py
@@ -17,7 +17,9 @@
 branch_labels: Union[str, Sequence[str], None] = None
 depends_on: Union[str, Sequence[str], None] = None
 
+__all__ = ["revision", "down_revision", "branch_labels", "depends_on"]
 
+
 def upgrade() -> None:
     """Upgrade schema."""
     # ### commands auto generated by Alembic - please adjust! ###
EOF
@@ -17,7 +17,9 @@
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None

__all__ = ["revision", "down_revision", "branch_labels", "depends_on"]


def upgrade() -> None:
"""Upgrade schema."""
# ### commands auto generated by Alembic - please adjust! ###
Copilot is powered by AI and may make mistakes. Always verify output.
revision: str = "19dc9714d9ea"
down_revision: Union[str, Sequence[str], None] = "11781e907181"
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None

Check notice

Code scanning / CodeQL

Unused global variable Note

The global variable 'depends_on' is not used.

Copilot Autofix

AI about 6 hours ago

To fix the problem while preserving existing functionality, we should keep the assignment (so the information and any potential Alembic expectations remain) but rename the variable so that its name clearly indicates it is intentionally unused and passes the static-analysis rule. The right-hand side is a simple literal (None), so there are no side effects to worry about.

The best minimal change in this file is to rename depends_on on line 18 to a name that contains unused, such as _unused_depends_on. Since the variable is not referenced elsewhere in the snippet, no other code changes are required. The rest of the file, including the other revision identifiers and the upgrade/downgrade functions, remains unchanged.

Concretely:

  • In alembic/versions/19dc9714d9ea_add_email_verification_model.py, update line 18 from depends_on: Union[str, Sequence[str], None] = None to _unused_depends_on: Union[str, Sequence[str], None] = None.
  • No additional imports, methods, or definitions are necessary.
Suggested changeset 1
alembic/versions/19dc9714d9ea_add_email_verification_model.py

Autofix patch

Autofix patch
Run the following command in your local git repository to apply this patch
cat << 'EOF' | git apply
diff --git a/alembic/versions/19dc9714d9ea_add_email_verification_model.py b/alembic/versions/19dc9714d9ea_add_email_verification_model.py
--- a/alembic/versions/19dc9714d9ea_add_email_verification_model.py
+++ b/alembic/versions/19dc9714d9ea_add_email_verification_model.py
@@ -15,7 +15,7 @@
 revision: str = "19dc9714d9ea"
 down_revision: Union[str, Sequence[str], None] = "11781e907181"
 branch_labels: Union[str, Sequence[str], None] = None
-depends_on: Union[str, Sequence[str], None] = None
+_unused_depends_on: Union[str, Sequence[str], None] = None
 
 
 def upgrade() -> None:
EOF
@@ -15,7 +15,7 @@
revision: str = "19dc9714d9ea"
down_revision: Union[str, Sequence[str], None] = "11781e907181"
branch_labels: Union[str, Sequence[str], None] = None
depends_on: Union[str, Sequence[str], None] = None
_unused_depends_on: Union[str, Sequence[str], None] = None


def upgrade() -> None:
Copilot is powered by AI and may make mistakes. Always verify output.
add VerificationToken model and migration for persistent token storage

implement class-based verification logic via VerificationTokenRepository and AuthVerificationService

add GET /api/v1/auth/verify-email with token validation, expiry handling, and idempotent verified-user behavior

add POST /api/v1/auth/resend-verification with enumeration-safe response and 3/minute rate limit

integrate signup to generate verification tokens and enqueue email dispatch via Kafka email producer

add standardized 429 error handling through custom rate-limit exception handler

document endpoints in docs/auth_verification_api.md

add tests for verification CRUD and API flows (success, missing token, invalid token, expired token, resend cases)

keep code quality gates compliant (ruff, isort, mypy) and auth tests green

Signed-off-by: aniebietafia <[email protected]>
@aniebietafia aniebietafia merged commit be15224 into main Mar 19, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Email Verification Endpoint

1 participant